some refactoring to the unit tests, adding cases for slurm #104
Conversation
tests/test_babs_check_setup.py
Outdated
|
|
||
| # TODO: should we just point to a dir in tmp_path? the assert should not be needed |
There was a problem hiding this comment.
The environmental variable TEMPLATEFLOW_HOME will be used by babs-init to generate the correct command for singularity run, where the path specified in TEMPLATEFLOW_HOME will be bound. Therefore this env var is needed. The assert here is just to make sure that the env var was set successfully in the line above assert. I understand this might seem a bit redundant to check, but that's easier for debugging (i.e., if it passes this assert, then the env var setting was not wrong).
babs/utils.py
Outdated
| @@ -16,6 +16,9 @@ | |||
| from datetime import datetime | |||
| import re | |||
|
|
|||
|
|
|||
| TYPE_SYSTEM_SUPPORTED = ['sge', 'slurm'] | |||
There was a problem hiding this comment.
This is a good idea! Do you mind moving this "constant" variable into babs/constants.py and importing it here? That python file holds several constant variables.
|
sorry, completely forgot that I was planning just remove some comments it's ready to merge, if you like it |
some refactoring of the tests and adding slurm
I've tried to slightly simplified the test functions and separate issues (e.g. asserts errors) that comes from potential issues in babs code and from the fact that someone added a testing case that is not covered by the examples of the configuration file.
I've also moved some some information to function arguments (and
pytest.mark.parametrize), so it's easier to see right away what is expected and what should be provided if more test cases are added.Of course, feel free to revert some/all the changes, I've also left a couple questions.